Repository navigation
MCP: admit calls chained on another's end, match only in-range stdio ids - #1666
Merged
Merged
Conversation
McpWorkers frees a call's permit before signalling its end again, so a call its subscribers start then is admitted with a single slot; a separate count keeps the call running until they are done, which awaitIdle() waits on, so stop() still ends listen streams cleanly. Over stdio, a numeric request id is a long only when it is exactly an integer in range, so an out-of-range cancellation no longer matches the largest long. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Lossy numeric ID conversion can cancel the wrong request, while worker errors can permanently inflate the running count and cause subsequent awaitIdle() calls to time out.
2 open issues
What changed in this PR
This PR addresses two MCP transport issues following #1665: chained calls rejected at the call limit and out-of-range stdio cancellation IDs matching unrelated requests.
Changes:
- Separates admission permits from worker completion tracking so chained calls can proceed while shutdown waits for end-signal subscribers.
- Uses decimal-based numeric ID normalization for stdio cancellation matching.
- Adds regression tests for chained calls, idle waiting, and out-of-range cancellation IDs.
| File | Description |
|---|---|
org.restlet.ext.mcp/src/test/java/org/restlet/ext/mcp/McpWorkersTestCase.java |
Adds chained-call and idle-wait regression tests. |
org.restlet.ext.mcp/src/test/java/org/restlet/ext/mcp/McpModernStdioTestCase.java |
Tests cancellation IDs beyond the long range. |
org.restlet.ext.mcp/src/main/java/org/restlet/ext/mcp/StdioServerHelper.java |
Changes numeric cancellation-key normalization. |
org.restlet.ext.mcp/src/main/java/org/restlet/ext/mcp/internal/McpWorkers.java |
Releases admission before signalling and tracks completion separately. |
Review effort: Balanced
A floating-point request id is compared by its exact binary value, not its shortest decimal form, so 1000000000000000128.0 matches its request. A call failing with an Error now ends with it and leaves the running count, so awaitIdle() no longer times out afterwards; the Error is rethrown once counted. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes two issues Copilot found in #1665 after it was merged.
Chained MCP calls refused with one slot (a regression in #1665).
McpWorkersfree a call's admission permit only after signalling the call's end. That stoppedstop()from writing an error after a listen stream's result.sendMessage(a).then(sendMessage(b))had its second send refused. That affects the HTTP and 2025-11-25 stdio client transports, which shareMcpWorkers.awaitIdle()waits on that count, so the listen-stream shutdown fix still holds.Erroralso ends with it and leaves that count, soawaitIdle()doesn't time out afterwards (found by Copilot here).An out-of-range numeric id cancelling another request over stdio.
notifications/cancelledto its request converted numeric ids throughlong.9223372036854775808.0therefore matched9223372036854775807.longonly when it is exactly an integer in thelongrange. Any other number is compared as a normalisedBigDecimal.1000000000000000128.0matches request1000000000000000128(found by Copilot here).Tests.
McpWorkersTestCase:3.0and passes with the fix;awaitIdle()waits until a call's end-signal subscribers are done;Errorends with it and no longer counts as running.McpModernStdioTestCase:9223372036854775808.0cancels nothing, then one naming9223372036854775807cancels that request;1000000000000000128.0cancels request1000000000000000128.mvn -pl org.restlet.ext.mcp verify: 73 tests, coverage and SpotBugs pass.🤖 Generated with Claude Code